Fix backwards compatability with `Lua.workspace.checkThirdParty` by 9999years · Pull Request #2406 · LuaLS/lua-language-server

9999years · 2023-11-07T17:16:57Z

I attempted to maintain backwards compatibility in #2354 but didn't fully understand the config type system. Here I checked for the booleans true and false:

lua-language-server/script/library.lua

Lines 611 to 617 in 3bbc8dd

    
           local checkThirdParty = config.get(uri, 'Lua.workspace.checkThirdParty') 
        
           -- Backwards compatability: `checkThirdParty` used to be a boolean. 
        
           if not checkThirdParty or checkThirdParty == 'Disable' then 
        
               return 
        
           elseif checkThirdParty == true then 
        
               checkThirdParty = 'Ask' 
        
           end

But the config value is now defined as a string:

lua-language-server/script/config/template.lua

Lines 320 to 325 in 5a763b0

    
           ['Lua.workspace.checkThirdParty']       = Type.String >> 'Ask' << { 
        
                                                       'Ask', 
        
                                                       'Apply', 
        
                                                       'ApplyInMemory', 
        
                                                       'Disable', 
        
                                                   },

So when the config is loaded as a boolean, it gets converted to a string:

lua-language-server/script/config/template.lua

Lines 81 to 85 in 5a763b0

    
           register('String', '', function (self, v) 
        
               return type(v) == 'string' 
        
           end, function (self, v) 
        
               return tostring(v) 
        
           end)

We can fix this by checking for the string values 'true' and 'false', rather than the boolean values true and false.

I attempted to maintain backwards compatability in LuaLS#2354 but didn't fully understand the config type system.

sumneko · 2023-11-08T02:17:02Z

Thank you!

pysan3 · 2023-11-08T12:53:57Z

Hi @9999years @sumneko ,

Reading the code, I believe when workspace.checkThirdParty is set to a boolean value, this code prevents from loading the value and fallbacks to default value in the first place.

lua-language-server/script/config/config.lua

Lines 56 to 71 in 8e880ad

    
           function m.setByScope(scp, key, value) 
        
               local unit = template[key] 
        
               if not unit then 
        
                   return false 
        
               end 
        
               local raw = m.getRawTable(scp) 
        
               if util.equal(raw[key], value) then 
        
                   return false 
        
               end 
        
               if unit:checker(value) then 
        
                   update(scp, key, unit:loader(value), value) 
        
               else 
        
                   update(scp, key, unit.default, unit.default) 
        
               end 
        
               return true 
        
           end

Since Type.String checks type(v) == 'string' as defined here.

lua-language-server/script/config/template.lua

Lines 81 to 85 in 8e880ad

    
           register('String', '', function (self, v) 
        
               return type(v) == 'string' 
        
           end, function (self, v) 
        
               return tostring(v) 
        
           end)

And it is an enum as well so util:checker also checks if the value is within the enum values, hence "false" does not work either.

lua-language-server/script/config/template.lua

Lines 27 to 44 in 8e880ad

    
           function mt:__shl(enums) 
        
               self.enums = enums 
        
               return self 
        
           end 
        
           function mt:checker(v) 
        
               if self.enums then 
        
                   local ok 
        
                   for _, enum in ipairs(self.enums) do 
        
                       if util.equal(enum, v) then 
        
                           ok = true 
        
                           break 
        
                       end 
        
                   end 
        
                   if not ok then 
        
                       return false 
        
                   end 
        
               end

Related: #2407

9999years · 2023-11-08T16:51:01Z

Hmm, this is unfortunate. I'm not sure if there's a way around this other than extending the configuration type system.

pysan3 · 2023-11-08T17:25:06Z

I guess if you want to preserve backwards compatibility, the best way would be to inject hardcoded converter inside this function.

lua-language-server/script/config/config.lua

Line 56 in 8e880ad

function m.setByScope(scp, key, value)

if key == ... and type(value) == "boolean" then
  value = value and "Apply" or "Disable"
end

Or, actually keep Lua.workspace.checkThirdParty key as before and introduce a new key Lua.workspace.checkThirdParty2 and change the behavior where it is referenced.

If it is hard to keep backwards compatibility I believe this should be announced as a breaking change since a lot of people depend on this option (at least in neovim community I hear a lot of people using setting this option to false before).

9999years · 2023-11-08T17:29:01Z

Yeah, I'll support reverting these changes until we can figure out the backwards compatibility story.

pysan3 · 2023-11-08T17:34:28Z

Actually, I do like your change and if we can keep backwards compat, I will be happy to have the option.

If there is a way to notify breaking changes, or we could somehow write a workaround, it'll be awesome but I'd like to hand it to the core developers.

sumneko · 2023-11-08T17:35:52Z

Have you tried Type.Or ?

@pysan3

I attempted to maintain backwards compatability in LuaLS#2354 and LuaLS#2406 but didn't fully understand the config type system. This approach uses `Type.Or` and I even tested it to confirm that it works! Thanks to @pysan3 for pointing out how a default would be used for invalid types and to @sumneko for pointing out `Type.Or`.

Fix backwards compatability with Lua.workspace.checkThirdParty

13a24ee

I attempted to maintain backwards compatability in LuaLS#2354 but didn't fully understand the config type system.

9999years mentioned this pull request Nov 7, 2023

Make workspace.checkThirdParty a string enum #2354

Merged

sumneko merged commit 8e880ad into LuaLS:master Nov 8, 2023

pysan3 mentioned this pull request Nov 8, 2023

feature: field type has changed in lua_ls config folke/neoconf.nvim#47

Closed

1 task

9999years deleted the checkthirdparty-back-compat branch November 8, 2023 16:50

9999years mentioned this pull request Nov 9, 2023

Fix backwards compatability with Lua.workspace.checkThirdParty (take 2!) #2409

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix backwards compatability with `Lua.workspace.checkThirdParty`#2406

Fix backwards compatability with `Lua.workspace.checkThirdParty`#2406
sumneko merged 1 commit intoLuaLS:masterfrom
9999years:checkthirdparty-back-compat

9999years commented Nov 7, 2023

Uh oh!

sumneko commented Nov 8, 2023

Uh oh!

pysan3 commented Nov 8, 2023 •

edited

Loading

Uh oh!

9999years commented Nov 8, 2023

Uh oh!

pysan3 commented Nov 8, 2023

Uh oh!

9999years commented Nov 8, 2023

Uh oh!

pysan3 commented Nov 8, 2023

Uh oh!

sumneko commented Nov 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	local checkThirdParty = config.get(uri, 'Lua.workspace.checkThirdParty')
	-- Backwards compatability: `checkThirdParty` used to be a boolean.
	if not checkThirdParty or checkThirdParty == 'Disable' then
	return
	elseif checkThirdParty == true then
	checkThirdParty = 'Ask'
	end

	['Lua.workspace.checkThirdParty'] = Type.String >> 'Ask' << {
	'Ask',
	'Apply',
	'ApplyInMemory',
	'Disable',
	},

	register('String', '', function (self, v)
	return type(v) == 'string'
	end, function (self, v)
	return tostring(v)
	end)

Uh oh!

Conversation

9999years commented Nov 7, 2023

Uh oh!

sumneko commented Nov 8, 2023

Uh oh!

pysan3 commented Nov 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

9999years commented Nov 8, 2023

Uh oh!

pysan3 commented Nov 8, 2023

Uh oh!

9999years commented Nov 8, 2023

Uh oh!

pysan3 commented Nov 8, 2023

Uh oh!

sumneko commented Nov 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pysan3 commented Nov 8, 2023 •

edited

Loading